-
-
Notifications
You must be signed in to change notification settings - Fork 460
Support .coveragerc.toml for configuration
#1952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Support .coveragerc.toml for configuration
#1952
Conversation
| (".coveragerc.toml", True, False), | ||
| ("setup.cfg", False, False), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the priority of the new config properly determined in the config_files_to_try function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
tests/test_config.py
Outdated
| @pytest.mark.parametrize("filename", ["pyproject.toml", ".coveragerc.toml"]) | ||
| def test_toml_config_file(self, filename) -> None: | ||
| # A pyproject.toml and coveragerc.toml will be read into the configuration. | ||
| self.make_file(filename, """\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the parameterization implemented appropriately, similar to pyproject.toml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
| def test_coveragerc_toml_priority(self) -> None: | ||
| """Test that .coveragerc.toml has priority over pyproject.toml.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is testing priority only over pyproject.toml in the test_coveragerc_toml_priority test sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
|
@nedbat, |
.coveragerc.toml for configuration.coveragerc.toml for configuration
nedbat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One change needed.
| (".coveragerc.toml", True, False), | ||
| ("setup.cfg", False, False), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
tests/test_config.py
Outdated
| @pytest.mark.parametrize("filename", ["pyproject.toml", ".coveragerc.toml"]) | ||
| def test_toml_config_file(self, filename) -> None: | ||
| # A pyproject.toml and coveragerc.toml will be read into the configuration. | ||
| self.make_file(filename, """\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
| def test_coveragerc_toml_priority(self) -> None: | ||
| """Test that .coveragerc.toml has priority over pyproject.toml.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
tests/test_config.py
Outdated
| """) | ||
| with mock.patch.object(coverage.tomlconfig, "has_tomllib", False): | ||
| msg = "Can't read 'pyproject.toml' without TOML support" | ||
| msg = "Can't read '{filename}' without TOML support" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like tests are failing because this needs to be an f-string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I will fix this error in the next commits. I run tox with version 3.12 locally, and I didn't see this error.
|
Thanks for doing this. There are some other changes needed. Let me know which you want to do, and which I should do:
|
Yes, I will add the changes |
4018790 to
673b0c0
Compare
673b0c0 to
c584e0d
Compare
webknjaz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plz revert all the formatting changes. I marked some, find the rest.
6bee75f to
7565729
Compare
7565729 to
2558209
Compare
CHANGES.rst
Outdated
| .. _pull 1952: https://github.com/nedbat/coveragepy/pull/1952 | ||
|
|
||
|
|
||
| .. start-releases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid conflict resolution?
doc/config.rst
Outdated
| [html] | ||
| directory = coverage_html_report | ||
| """, | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stray edit?
tests/test_config.py
Outdated
| """\ | ||
| @pytest.mark.parametrize("filename", ["pyproject.toml", ".coveragerc.toml"]) | ||
| def test_toml_config_file(self, filename: str) -> None: | ||
| # A pyproject.toml and coveragerc.toml will be read into the configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # A pyproject.toml and coveragerc.toml will be read into the configuration. | |
| # A pyproject.toml and .coveragerc.toml will be read into the configuration. |
tests/test_config.py
Outdated
| def test_toml_parse_errors(self, bad_config: str, msg: str) -> None: | ||
| @pytest.mark.parametrize("filename", ["pyproject.toml", ".coveragerc.toml"]) | ||
| @pytest.mark.parametrize("bad_config, msg", [ | ||
| ("[tool.coverage.run]\ntimid = \"maybe?\"\n", r"maybe[?]"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of reformatting?
tests/test_config.py
Outdated
| assert cov.config.xml_output == "/Users/me/somewhere/xml.out" | ||
| assert cov.config.exclude_list == ["~/data.file", "~joe/html_dir"] | ||
| assert cov.config.paths == {"mapping": ["/Users/me/src", "/Users/joe/source"]} | ||
| assert cov.config.paths == {'mapping': ['/Users/me/src', '/Users/joe/source']} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
tests/test_config.py
Outdated
| self.make_file( | ||
| "pyproject.toml", | ||
| """\ | ||
| self.make_file("pyproject.toml", """\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
2558209 to
05be667
Compare
This adds .coveragerc.toml to the list of configuration files that coverage.py will automatically search for, with higher priority than pyproject.toml but lower than .coveragerc.
Converts existing pyproject.toml tests to be parametrized to also test .coveragerc.toml functionality, ensuring both TOML config formats work identically.
This test ensures that when both .coveragerc.toml and pyproject.toml are present, .coveragerc.toml takes precedence as intended.
…agerc.toml configuration file support
05be667 to
0a508bf
Compare
|
I'm in progress - some tests have failed, I'm going to fix them |
| # A pyproject.toml file will be read into the configuration. | ||
| @pytest.mark.parametrize("filename", ["pyproject.toml", ".coveragerc.toml"]) | ||
| def test_toml_config_file(self, filename) -> None: | ||
| # A pyproject.toml and .coveragerc.toml file will be read into the configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, not both, just one of them, right?
| # A pyproject.toml and .coveragerc.toml file will be read into the configuration. | |
| # A pyproject.toml or .coveragerc.toml file will be read into the configuration. |
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <[email protected]>
|
I'm very excited to start using this feature, thanks for the effort here @OlenaYefymenko!!! |
|
@webknjaz @OlenaYefymenko What is left to do with this PR? |
|
@nedbat just the change log move. Olena was going to mark is as ready just about now. |
@nedbat the pull request is ready for review. Please let me know if anything needs to be edited. |
|
Thanks for doing this, and thanks for persisting. With these changes, the [tool.coverage.run]
branch = truebut wouldn't we want this?: [run]
branch = trueI'd like to make things seem natural, and I also like to give people options, so it's hard to strike the right balance. When I read (for example) pytest's explanation of their configuration rules my head starts to spin. |
I've seen both startegies in the wild and I'm leaning towards not having differences with For example, Towncrier has I did mention this opinion in the original issue, FTR: #1643. My reasoning for this is that it's easier to migrate/move things between
Right. I missed the PR it when was being implemented.. I'd ask for a prefix if I'd seen it, but there's no going back I guess. Ruff has the section/prefix removed, OTOH: https://docs.astral.sh/ruff/configuration/#__tabbed_1_2. It does save a few keystrokes but I'd rather keep it uniform. |
|
Please, please do not support the
@OlenaYefymenko Would you be open to such a change in this PR? |
|
That's up to @nedbat to decide, then. |
|
Oh, for sure! I was just asking if @OlenaYefymenko would personally be fine with that approach. |
|
I hope this doesn't seem like a cop-out: I think there are good arguments for both side, and I'd like to support both prefixed and un-prefixed settings, aiming for the least surprise. The only complexity here is that in pyproject.toml, we need to insist on the prefix. We already have precedent for this. In tomlconfig.py we have this: prefixes = ["tool.coverage."]
for prefix in prefixes:
real_section = prefix + sectionThis code looks silly (why iterate over a fixed one-element list?), but it's that way because config.py has this: class HandyConfigParser(configparser.ConfigParser):
def __init__(self, our_file: bool) -> None:
"""Create the HandyConfigParser.
`our_file` is True if this config file is specifically for coverage,
False if we are examining another config file (tox.ini, setup.cfg)
for possible settings.
"""
self.section_prefixes = ["coverage:"]
if our_file:
self.section_prefixes.append("")That code is there because we insist on the "coverage:" prefix in tox.ini and setup.cfg, but are flexible in .coveragerc or in an explicitly specified config file. It would take just adding two lines to tomlconfig.py to let it read either style of section, while being strict about the sections in pyproject.toml. To avoid over-complicating the docs, I'd mention the flexibility of section names in the opening paragraphs about syntax, but in the examples, only show the fully prefixed syntax. Maybe the tab name could be just "toml"? I hope everyone will be happy with this approach. |
|
Sounds reasonable. |
|
I think it would be good to mention the possibility of either naming in the opening but I disagree about showing the fully prefixed syntax in the examples because then that would encourage folks to copy/paste that forever. I would imagine that there would be a new tab that comes first here:
This new tab would have the name
Making the new standalone TOML file the one that is shown by default as well as its syntax would then maintain complete compatibility with the current documentation that starts the sections with the name of the options and no prefix:
This would also match strategy of the In any case even if there is disagreement here about the preferred file format I think the prefix should not be prominent when, like I mentioned, it really is just an artifact of a packaging quirk in our specific language. I'm heavily involved in Python packaging now but I wasn't around when that was happening, so I'm sorry about that. |
|
Agreed, a dedicated tab sounds good. That's sounds closer to what the original feature request was after. |
I requested that name originally since it sounded more familiar. The precedent is Also, wouldn't |
Good points! |
|
This is slightly off topic, but since it came up: I prefer not to hide configuration files in my repositories. In addition to I'd be happy to take my bike shedding to a new issue so as to not derail this pull request. Thanks for your effort on this! |
|
@serban many people prefer the opposite — not seeing many files in repo roots. That's why a lot of them almost religiously dump hundred of lines into |
| assert cov.config.data_file == ".toml-data.dat" | ||
| assert cov.config.branch is True | ||
|
|
||
| def test_coveragerc_toml_unprefixed_syntax(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be a good idea to also add a test making sure that unprefixed sections are ignored if present in pyproject.toml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, thank you. I will add it and will also need to fix the linting errors



This patch provides a new configuration —
.coveragerc.toml. Considering that many projects have switched to toml configurations, this change offers a more flexible approach to manage coverage settings.Resolves #1643